Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reveal trait and its implementations for ProtocolContext #223

Merged
merged 4 commits into from
Nov 15, 2022

Conversation

akoshelev
Copy link
Collaborator

@akoshelev akoshelev commented Nov 12, 2022

Another step to make sort protocol support both semi-honest and malicious security.

In semi-honest setting, CheckZero and ConvertShares will use semi-honest reveal with a cost of 1 multiplication. Malicious reveal currently has a cost of 2 multiplications and will be used only for malicious setting.

This change leverages recently stabilized GAT. If we like this implementation, SecureMul will be changed accordingly. I tried it and I don't think it works for multiplication

Another step to make sort protocol support both semi-honest and malicious security.

In semi-honest setting, `CheckZero` and `ConvertShares` will use semi-honest reveal with a cost of 1 multiplication. Malicious reveal currently has a cost of 2 multiplications and will be used only for malicious setting.

This change leverages recently stabilized GAT (https://blog.rust-lang.org/2022/10/28/gats-stabilization.html). If we like this implementation, `SecureMul` will be changed accordingly.abilized GAT (https://blog.rust-lang.org/2022/10/28/gats-stabilization.html). If we like this implementation, `SecureMul` will be changed accordingly.
It drops `SecurityValidator` so it is not really that useful. Reveal does not care, so it can use it
share(input, rng)
.iter()
.enumerate()
.map(|(i, share)| MaliciousReplicated::new(*share, r[i]))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t think this is right. The second argument should be a sharing of input times r. This appears be be just r.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that you want:

let r = validate_and_reconstruct((r[0], r[1], r[2]));
let rx = r * input;
zip(share(input, rng), share(rx, rng))
  .map(|(x, rx)| MaliciousReplicated::new(*x, *rx))
  .collect::<Vec<_>>()
  .try_into()
  .unwrap()

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep that was a bug. I decided to generate r inside share_malicious given that I have access to PRNG there. Lmk if that is not ergonomic (we could always reconstruct $r$ from $[rx]$)

Copy link
Collaborator

@benjaminsavage benjaminsavage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Just one conflict and the issue with the test fixture (which works ATM because you’re not really using any properties of the malicious replicated secret sharing in these particular tests, but which should still be resolved)

.iter()
.zip(share(rx, rng))
.map(|(x, rx)| MaliciousReplicated::new(*x, rx))
// TODO: each_ref when stable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does each_ref() work with zip?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should work if array::zip is stable rust-lang/rust#80094. I'll mention both in the todo

@akoshelev
Copy link
Collaborator Author

sorry had to force push as I screwed up during merge and it is quite painful to do it again :(

@martinthomson martinthomson merged commit 7565647 into private-attribution:main Nov 15, 2022
@akoshelev akoshelev deleted the reveal-trait branch November 15, 2022 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants